Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix bug around ref resolution of type mentioned in type arguments #3768

Merged
merged 1 commit into from
May 10, 2024
Merged

Fix bug around ref resolution of type mentioned in type arguments #3768

merged 1 commit into from
May 10, 2024

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented May 10, 2024 •
edited
Loading

Fixes #3758

The main fix is in method.dart, when finding "elements nearby" a method, we were including the ElementType for things like "type arguments in the return type," as in Vehicle in Future<Vehicle>. But the ElementType does not have an href; it has a ModelElement which does have an href.

I also simplify a few things:

  • Also in method, we were looking at the "type arguments" of a method declartion, but this is implemented in the ElementType hierarchy to be const []. Oy. OK, removed.
  • A recent refactor meant that allowTree was only ever (_) => true, so I scrapped that parameter in a few functions, simplfying everything.
  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@srawlins srawlins requested a review from kevmoo May 10, 2024 16:07
@srawlins
Copy link
Member Author

flutter bot still failing with "dartdoc 8.0.8 (/home/runner/work/dartdoc/dartdoc/.dart_tool/pub/bin/dartdoc/dartdoc.dart-3.5.0-138.0.dev.snapshot) failed: Command executables must exist. The file "/tmp/flutterNMVXCR/bin/cache/artifacts/snippets/snippets" does not exist for tool snippet." unrelated to this change.

@srawlins srawlins requested review from bwilkerson and kallentu May 10, 2024 16:12
@srawlins
Copy link
Member Author

Hey @bwilkerson @kallentu , my initial plan with Kallen's move was to keep sending her reviews, as she is currently the person with the 2nd most dartdoc knowledge. But I think it's actually important to bring that knowledge back in-house, as it were, so I need a new primary reviewer on the analyzer team :D I added you @bwilkerson sort of as a default, but we can discuss who should primarily get dartdoc reviews.

kallentu
kallentu approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@kallentu kallentu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this part of the lookup more readable than it was.

Regarding spreading the knowledge in the analyzer team, I think that's a great idea! Would take a lot of weight off your shoulders too. I'm happy to still be cc'd on reviews though. I have good amount of vested interest in Dartdoc now haha

lib/src/model/method.dart
// method, this is woefully incomplete. Notice we don't currently include
// the element of the returned type itself, nor nested type arguments,
// nor other nested types e.g. in the case of function types or record
// types. But this is all being replaced with analyzer's resolution soon.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
I'm excited to tear all of this out with the analyzer's resolution.

@srawlins srawlins merged commit 2e706be into dart-lang:main May 10, 2024
6 of 7 checks passed
@srawlins srawlins deleted the fix-type-arg-resolution branch May 10, 2024 21:04
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request May 13, 2024
@devoncarew
[deps] rev dartdoc, ecosystem, fixnum, glob, html, http, http_multi_s…
d4f3288 
…erver, http_parser, json_rpc_2, markdown, matcher, mockito, package_config, path, pool, pub_semver, shelf, test, test_descriptor, test_process, test_reflective_loader, web, web_socket_channel, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/6e9c1ea..2e706be):
  2e706be8  2024-05-10  Sam Rawlins  Fix bug around ref resolution of type mentioned in type arguments (dart-lang/dartdoc#3768)
  d4a5f0b3  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/dartdoc#3766)

ecosystem (https://github.com/dart-lang/ecosystem/compare/9ce560b..5890417):
  5890417  2024-05-10  Parker Lougheed  Update dart_flutter_team_lints to depend on lints v4 (dart-lang/ecosystem#263)

fixnum (https://github.com/dart-lang/fixnum/compare/2c9f25d..ac892ad):
  ac892ad  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/fixnum#128)

glob (https://github.com/dart-lang/glob/compare/44e8c22..ee48ea8):
  ee48ea8  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/glob#93)
  c6362fb  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/glob#92)

html (https://github.com/dart-lang/html/compare/44613e8..00d3461):
  00d3461  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/html#245)
  e676b96  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/html#244)

http (https://github.com/dart-lang/http/compare/dd31e64..fdb0d36):
  fdb0d36  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/http#1194)

http_multi_server (https://github.com/dart-lang/http_multi_server/compare/e45a674..4a791af):
  4a791af  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/http_multi_server#69)
  ff49aed  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/http_multi_server#68)

http_parser (https://github.com/dart-lang/http_parser/compare/ad05810..702698a):
  702698a  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/http_parser#91)
  d900d8e  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/http_parser#90)

json_rpc_2 (https://github.com/dart-lang/json_rpc_2/compare/7547bb1..3187f7b):
  3187f7b  2024-05-08  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/json_rpc_2#114)
  94b6c49  2024-05-08  Devon Carew  blast_repo fixes (dart-archive/json_rpc_2#113)

markdown (https://github.com/dart-lang/markdown/compare/6aaa152..7463999):
  7463999  2024-05-07  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/markdown#610)
  a2313a5  2024-05-07  Devon Carew  blast_repo fixes (dart-lang/markdown#609)

matcher (https://github.com/dart-lang/matcher/compare/b42bf10..4ac4096):
  4ac4096  2024-05-07  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/matcher#248)
  f22df8a  2024-05-07  Devon Carew  blast_repo fixes (dart-archive/matcher#247)

mockito (https://github.com/dart-lang/mockito/compare/81ecb88..4be52e1):
  4be52e1  2024-05-07  Devon Carew  blast_repo fixes (dart-lang/mockito#749)

package_config (https://github.com/dart-lang/package_config/compare/a36e496..3909676):
  3909676  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/package_config#154)
  ca40813  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/package_config#153)

path (https://github.com/dart-lang/path/compare/f411b96..9be79e7):
  9be79e7  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/path#164)
  283f31a  2024-05-09  Devon Carew  blast_repo fixes (dart-archive/path#163)

pool (https://github.com/dart-lang/pool/compare/e6df05a..1a6f2df):
  1a6f2df  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/pool#86)
  f2f8a8b  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/pool#85)

pub_semver (https://github.com/dart-lang/pub_semver/compare/df834e1..f57c9c3):
  f57c9c3  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/pub_semver#103)
  2071731  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/pub_semver#102)

shelf (https://github.com/dart-lang/shelf/compare/c8d8058..d9f82bf):
  d9f82bf  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/shelf#431)
  96d7283  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/shelf#430)
  468e2d9  2024-05-06  Brian Quinlan  Remove `dependency_overrides` (dart-lang/shelf#429)
  9e57c33  2024-05-06  Brian Quinlan  Prepare shelf_web_socket 2.0 for release (dart-lang/shelf#425)

test (https://github.com/dart-lang/test/compare/0562baf..84d2a2b):
  84d2a2bc  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test#2220)
  b7fa27d5  2024-05-07  Nate Bosch  Point README links to the test library (dart-lang/test#2219)
  d1e27677  2024-05-06  Brian Quinlan  Allow the next semver major versions of shelf_web_socket and web_socket_channel (dart-lang/test#2204)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/6695954..d61bf6c):
  d61bf6c  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_descriptor#66)
  2861ba3  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_descriptor#65)

test_process (https://github.com/dart-lang/test_process/compare/69c6894..4ab3f1c):
  4ab3f1c  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_process#58)
  6efaf7f  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_process#57)

test_reflective_loader (https://github.com/dart-lang/test_reflective_loader/compare/537ab5b..f8807e0):
  f8807e0  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_reflective_loader#62)
  1c62cd9  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_reflective_loader#61)

web (https://github.com/dart-lang/web/compare/ff7185c..c36d962):
  c36d962  2024-05-12  mnordine  Fix typo (dart-lang/web#236)

web_socket_channel (https://github.com/dart-lang/web_socket_channel/compare/d86313d..5f65bae):
  5f65bae  2024-05-06  Brian Quinlan  Remove dependency_overrides (dart-lang/web_socket_channel#354)
  324d932  2024-05-06  Brian Quinlan  Make shelf_web_socket a dev dependency (dart-lang/web_socket_channel#353)
  0eca60d  2024-05-06  Brian Quinlan  Remove "wip" label for 3.0 release (dart-lang/web_socket_channel#347)

webdev (https://github.com/dart-lang/webdev/compare/c233e45..9108903):
  91089031  2024-05-10  Elliott Brooks  Update CONTRIBUTING doc for DWDS (dart-lang/webdev#2429)
  635b0d9d  2024-05-10  Elliott Brooks  Delete MV2 Dart Debug Extension (dart-lang/webdev#2428)
  ae43e8e9  2024-05-09  Elliott Brooks  Remove all unsound tests (dart-lang/webdev#2426)
  239cb7f3  2024-05-08  Elliott Brooks  Register service extensions on a client that is connected to DDS  (dart-lang/webdev#2388)

Change-Id: Ic0de7f6e41b4e93e3d26b7d1a74fe012bc7b086e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366161
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kallentu kallentu kallentu approved these changes

@kevmoo kevmoo Awaiting requested review from kevmoo

@bwilkerson bwilkerson Awaiting requested review from bwilkerson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Class references not working in method doc strings
2 participants
@srawlins @kallentu